Replace pandas-based LP file writing with polars implementation #496
Conversation
- Remove pandas-based LP writing functions and replace with polars versions - Rename polars functions to remove '_polars' suffix for consistent API - Create separate get_printers_scalar() for non-LP functions (highspy, gurobi, mosek) - Update get_printers() to handle polars dataframes for LP writing - Consolidate "lp" and "lp-polars" io_api options to use same implementation - Remove unused imports and cleanup handle_batch function
|
the replacement comes with a small trade off for smaller problems where the pandas based IO is still a bit faster. however for larger problems polars significantly speeds up. any opinios @coroa @lkstrp ? (note that the line of the crossover point in the upper right is a bit off, but you get the message) |
|
also tagging @fneum |
|
thinking about it I would say we go for it. we are talking about maximally 7 ms slower in bad configurations (tiny problems) but minutes faster for large problems, and the code get streamlined. |
lkstrp
left a comment
lkstrp
left a comment
There was a problem hiding this comment.
thinking about it I would say we go for it. we are talking about maximally 7 ms slower in bad configurations (tiny problems) but minutes faster for large problems, and the code get streamlined.
Agreed! Also the cleanup is nice
The polars migration broke NaN validation because check_has_nulls_polars only checked for null values, not NaN values. In polars, these are distinct concepts. This fix enhances the validation to detect both null and NaN values in numeric columns while avoiding type errors on non-numeric columns. Fixes failing tests in test_inconsistency_checks.py that expected ValueError to be raised when variables have NaN bounds.
|
Agree as well, but we should do a memory check comparison as well (e.g. with a PyPSA-EUR case). |
good point, but no need to worry as we have the slicing logic which ensures that memory requirements are bound |
|
Maybe someone can do it nevertheless? I am not sure if anyone ever tested the polars writing implementation on PyPSA-Eur type large problem (even if it has same slicing logic). |
|
Anyone knows whether the polars code here is within the confines of the narwhals compat layer (which is a subset of the full polars api)? Then the switching between pandas and polars could easily be made a config option. And atlite does not have to depend on polars, but it could remain optional. |
|
@FabianHofmann @coroa Great Work. |
|
The PR was breaking though, and the release should have been Also filter_nulls Line 631 in 6d1302e only checks for "empty" constraints (label is -1 or coeff is 0) but not for actual nans. While the line below not just treats "empty" constraints and raise actually nans:Line 632 in 6d1302e @FabianHofmann Could you point me at a fix? |
Closes # (if applicable).
Changes proposed in this Pull Request
Checklist
doc.doc/release_notes.rstof the upcoming release is included.